Skip to content

Reduction workflow scripts - #129

Open
ambergorzynski wants to merge 15 commits into
mainfrom
reduction-scripts
Open

Reduction workflow scripts#129
ambergorzynski wants to merge 15 commits into
mainfrom
reduction-scripts

Conversation

@ambergorzynski

Copy link
Copy Markdown
Collaborator

Adds a reduction workflow as the third pipeline stage after gap filling: scaffold and optionally run llvm-reduce on the first N gap-fill hits from incremental/new_coverage.csv.

  • Refactors batch reduction into src/reduce/batch/ with a python -m reduce batch-from-coverage CLI; removes legacy per-target reduce scripts and batch_reduce_using_coverage.py.
  • Adds scripts/reduction.sh and scripts/docker/reduction.sh entrypoints, plus shared bash helpers under scripts/lib/.
  • Adds scripts/docker/run-full-workflow.sh to smoke-test gap finding → gap filling → reduction in Docker.
  • Adds unit tests, a scaffold-only lit test (reduction-batch.test), and an optional e2e reduction extension (--param e2e_reduce=1 on the PR gap-finding test).
  • Documents the reduction workflow in the README and wires llvm-dis into the Docker image for .bc inputs.

Split scaffolding logic into focused modules (coverage, candidate_test,
templates, pipeline, prepare) and pass parsed test.sh info into
prepare_test_case to avoid parsing each row twice.
…flow.sh

Enable end-to-end Docker testing with real fuzz corpora by skipping host
LLVM validation in containers, installing llvm-reduce in the image, and
resolving llvm-dis for .bc inputs. Add CORPUS env support, rename
test-workflow.sh, and document the workflow in the README.
Gate llvm-reduce behind --param e2e_reduce=1, verify reduced.ll exists
and is smaller than the disassembled input without pinning llvm-reduce output.
Comment thread integration-tests/e2e/gap-finding-pr/pr-214457.test Outdated
# RUN: %FileCheck %s --input-file %t/out-n1/t-00001-test-a/config.json --check-prefix=CFG1
# RUN: %FileCheck %s --input-file %t/out-n1/t-00001-test-a/interesting_ir.sh --check-prefix=IR1

# RUN: %reduce batch-from-coverage --csv %t/gap-fill/incremental/new_coverage.csv --candidate-tests %t/gap-fill/candidate_tests --output %t/out-n3 --template-dir %repo-root/example/amd/new-test-1 --n 3

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference between this call and the one above? Could we have just one?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference is the number of tests - I am testing reducing 1 or reducing 3. I guess we could just have the reduce 3 if you want to simplify?

# COM: Scaffold-only batch-from-coverage for n=1 and n=3 (no llvm-reduce/creduce).

# RUN: rm -rf %t && mkdir -p %t
# RUN: bash %S/fixtures/reduction-batch/prepare-gap-fill-fixture.sh %t/gap-fill

@mgcarrasco mgcarrasco Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to run it on a real but small case?

We could run a simple baseline with a one-test filter, run one candidate test, this will necessarily identify "gaps", and we reduce in regard to that.

In this way we are testing a real scenario without having to maintain prepare-gap-fill-fixture.sh. It is creating many stubs, that if any other component is updated, the stub generation would have to be updated as well.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a real case in the e2e pipeline - do you mean something else?

The prepare-gap-fill-fixture.sh is to test batch reduction, where we reduce many tests rather than just 1, so it is testing something a bit different.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be handy to make sure that tool is performing a true reduction (not using a fake llc) without having to analyze a PR. Also, while making it real it could also reduce the boilerplate around the test (i.e., prepare-gap-fill-fixture.sh)

The prepare-gap-fill-fixture.sh is to test batch reduction, where we reduce many tests rather than just 1, so it is testing something a bit different.

The integration test should be a real execution of the tool. This test is not only stubbing some initial files but almost the entire reduction. prepare-gap-fill-fixture.sh prepares a fake llc. The integration tests for finding and filling are executed in the LLVM build that they have available.

Comment thread integration-tests/reduction-batch.test
Comment thread integration-tests/lit.cfg.py Outdated

@mgcarrasco mgcarrasco Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a new one or we could use an existing one under integration-tests?

Comment thread scripts/lib/reduction-local.sh
@@ -0,0 +1,207 @@
"""Unit tests for batch-from-coverage scaffolding."""

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this testing something that is not tested by the integration tests?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not substantially, agree we only need one. I would prefer keeping the unit tests since they are more targeted and test some specific things. Do you have a preference?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd make the integration test reduce the test(s) in a more realistic setting and then delete the unit test. I find it easier to read and follow.

Comment thread integration-tests/e2e/gap-finding-pr/pr-214457.test Outdated
@mgcarrasco

Copy link
Copy Markdown
Collaborator

Regarding the new functionality, my main feedback so far is on the interface. It feels driven by fuzz-fill’s internals rather than user tasks or domain concepts, perhaps. What’s the unit of work that the new functionality processes?

The interface appears to handle many details simultaneously, and the interaction of the commands could be difficult to follow. I understand that this is also influenced by how the "core" reduce currently works.

Do you think it could be improved? If so, what would be for you the axes or fundamental steps of the process? On one hand, we have the many different reduction tactics you explored, possibly useful if fuzz-fill is used “raw”. Then, we have the “simplest” reduction that we can plug after the gap filling flow.

If we go back to the core reduction logic, what are the possible ways in which we can reduce a test (IR/MIR) for llc?

I'm also asking this to brainstorm it and refresh my memory after some months of your initial work on reduction.

Maybe the batch and core reduction could be broken into smaller steps.

Comment thread scripts/lib/reduction.sh
@@ -0,0 +1,44 @@
#!/usr/bin/env bash

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering whether it is worth adding considering that it is not providing anything on top of python -m reduce batch-from-coverage. Do you think this is true?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the other hand, it makes it more coherent with finding and filling.

@@ -0,0 +1,331 @@
#!/usr/bin/env bash

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to understand whether this CLI parsing logic is already in the Python module. If it is, would it better to let the validation happen in Python?

@mgcarrasco

Copy link
Copy Markdown
Collaborator

@ambergorzynski just to follow, does the reduction at the moment support multiple instrumentation points? if not, could we add a ticket for it?

@mgcarrasco

Copy link
Copy Markdown
Collaborator

@ambergorzynski this is my summary of what we discussed yesterday as a follow up for the reduction CLI. Let me know what you think, and if I'm missing anything.

There are two concepts here, one is the reduction that fuzz-fill has, and on the other hand is how that is used in the context of analyzing PRs (or workflows in general).

fuzz-fill can reduce either MIR, IR and has pipelines for the reduction based on the tactics you explored. It could be handy if the CLI allows specifying:

  • A single file and its instrumentation points
  • A list of (file, instrumentation points)
  • A config for the other settings that remain the same for all input files

For PRs, for now at least, we are just interested in reducing the IR modules in the simplest way possible. Conceptually, this is a list of (file, line) under the same setting. Assuming we have the above CLI in fuzz-fill, the PR analysis only has to convert the lines to instrumentation points.

I guess the above design would then completely remove the "batch" processing we currently have. Something I feel that is missing is that the reduction in fuzz-fill wouldn't be that friendly in terms of consuming the artifact that candidate tests generated.

By any chance, just for documentation, do you have at hand a very brief summary of the reduction tactics you explored? I mean for example, I remember that in some cases we extracted the IR after certain pass, that was not necessarily the one under test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants